chore: merge main into development - #147
Conversation
…ubmit Accept Story music jobs only after persisting command/task/candidate IDs. The same idempotency key and spec replay; a reused key with a different spec conflicts. Retry/new-version mint a new attempt. Existing 202 jobs and the sync compatibility route stay.
Default remains a labeled fast pre-push mode. --full runs the CI-safe guards, pytest suite, ratchet, UI checks, budget and simulated E2E. A missing ratchet base or a failed analyzer is now a hard failure.
Technical review belongs to agents; a human merge click is operational. Document the intended required checks and the admin steps that still need authorization. Do not change remote protection in this PR.
bash -lc could replace nvm/Pinokio PATH after require_cmd npm. Use a non-login helper so the same npm binary is invoked.
An AND list under set -e does not abort. Run build and budget as separate steps so a failed build never reaches E2E or a CI-equivalent PASS.
Omit a key to start a new attempt. Retry needs its own key so a lost response can replay. Replay without a MiniMax checkpoint starts the worker again. Default candidate count matches the jobs endpoint.
Insert the in-memory job under the existing lock; if the reserved job_id is already live, return that snapshot instead of starting a second provider thread.
feat: reserve music generation IDs before inference with idempotent submit
fix(ci): make local validation explicit and fail closed
docs: record agent QA policy without applying GitHub rulesets
|
cursor review |
PR Review — Loreframe StudioRisk: low Automated review from Findings
Changed files
CONTRIBUTING checklist
Posted by the repo PR review workflow. Re-runs on each push to the PR. |
Code healthQuality score: 49.9/100Higher is better. The score is a trend dashboard; the independent ratchet below remains the CI gate.
Change vs PR base: +0.1 points.
Markdown, JSON catalogs and tests are out of this table. Only Most complex functions
Trend vs baseline
Warnings
Ratchet passed. |
There was a problem hiding this comment.
Cursor Bugbot has reviewed your changes using high effort and found 1 potential issue.
Bugbot Autofix prepared a fix for the issue found in the latest run.
- ✅ Fixed: Publish wipes reserved task identity
- El upsert ahora fusiona metadata reservada y conserva workflow/title, y el publish de MiniMax proyecta generation_id, candidate_id, command_id e idempotency_key desde el job.
Or push these changes by commenting:
@cursor push 90761cd2ee
Preview (90761cd2ee)
diff --git a/app/_launch_runtime.py b/app/_launch_runtime.py
--- a/app/_launch_runtime.py
+++ b/app/_launch_runtime.py
@@ -35994,6 +35994,22 @@
# cancellation/completion won. Canonical resume explicitly transitions
# the registry before the next active adapter snapshot arrives.
return existing
+ existing_metadata = existing.get("metadata") if isinstance(existing.get("metadata"), dict) else {}
+ incoming_metadata = fields.get("metadata") if isinstance(fields.get("metadata"), dict) else None
+ if incoming_metadata is not None:
+ merged_metadata = dict(existing_metadata)
+ for key, value in incoming_metadata.items():
+ if value is not None:
+ merged_metadata[key] = value
+ fields["metadata"] = merged_metadata
+ # Reservation owns attempt identity. Adapter snapshots must not
+ # replace workflow/title after submit_music_generation wrote them.
+ if any(
+ existing_metadata.get(key)
+ for key in ("generation_id", "candidate_id", "command_id", "idempotency_key")
+ ):
+ fields.pop("workflow", None)
+ fields.pop("title", None)
mutable = {
key: value for key, value in fields.items()
if key not in {"id", "created_at"}
@@ -36328,9 +36344,16 @@
"actor": provenance.get("actor") or "unknown",
"tool": provenance.get("tool") or adapter,
"capability": provenance.get("capability"),
- "command_id": command.get("command_id"),
+ "command_id": (
+ command.get("command_id")
+ or record.get("commandId")
+ or record.get("command_id")
+ ),
"workflow_id": command.get("workflow_id"),
"run_id": command.get("run_id"),
+ "generation_id": record.get("generationId") or record.get("generation_id"),
+ "candidate_id": record.get("candidateId") or record.get("candidate_id"),
+ "idempotency_key": record.get("idempotencyKey") or record.get("idempotency_key"),
},
)
diff --git a/tests/test_task_adapter_helpers.py b/tests/test_task_adapter_helpers.py
--- a/tests/test_task_adapter_helpers.py
+++ b/tests/test_task_adapter_helpers.py
@@ -256,3 +256,81 @@
assert captured["id"] == "task-model3d-backend-id"
assert captured["root_id"] == "task-series-root"
+
+
+def test_generic_adapter_publishes_reserved_music_identity():
+ publish, captured = _load_publisher("_publish_generic_legacy_task")
+
+ publish({
+ "jobId": "minimax-music-abc123def456",
+ "taskId": "task-minimax-music-abc123def456",
+ "workspace": "default",
+ "status": "queued",
+ "generationId": "gen-1",
+ "commandId": "cmd-1",
+ "candidateId": "song-1",
+ "idempotencyKey": "idem-1",
+ }, "minimax-music")
+
+ assert captured["metadata"]["generation_id"] == "gen-1"
+ assert captured["metadata"]["command_id"] == "cmd-1"
+ assert captured["metadata"]["candidate_id"] == "song-1"
+ assert captured["metadata"]["idempotency_key"] == "idem-1"
+
+
+def test_upsert_keeps_reserved_task_identity():
+ existing = {
+ "id": "task-minimax-music-abc",
+ "status": "queued",
+ "workflow": "generate_story_song",
+ "title": "Story song",
+ "metadata": {
+ "generation_id": "gen-1",
+ "candidate_id": "song-1",
+ "command_id": "cmd-1",
+ "idempotency_key": "idem-1",
+ },
+ }
+ updated = {}
+
+ class Registry:
+ def get(self, _task_id):
+ return existing
+
+ def update(self, task_id, **fields):
+ updated.update(fields)
+ return {**existing, **{key: value for key, value in fields.items()
+ if key not in {"force", "event_type", "event_exclude_fields"}}}
+
+ namespace = {
+ "_task_registry": lambda _workspace: Registry(),
+ }
+ node = _function("_upsert_canonical_task")
+ exec(compile(ast.Module(body=[node], type_ignores=[]), str(LAUNCH_PATH), "exec"), namespace)
+
+ result = namespace["_upsert_canonical_task"](
+ "default",
+ "task-minimax-music-abc",
+ workflow="minimax-music",
+ title="MiniMax Music",
+ status="queued",
+ metadata={
+ "adapter": "minimax-music",
+ "actor": "unknown",
+ "tool": "minimax-music",
+ "capability": None,
+ "command_id": None,
+ "workflow_id": None,
+ "run_id": None,
+ },
+ )
+
+ assert "workflow" not in updated
+ assert "title" not in updated
+ assert result["workflow"] == "generate_story_song"
+ assert result["title"] == "Story song"
+ assert result["metadata"]["generation_id"] == "gen-1"
+ assert result["metadata"]["candidate_id"] == "song-1"
+ assert result["metadata"]["command_id"] == "cmd-1"
+ assert result["metadata"]["idempotency_key"] == "idem-1"
+ assert result["metadata"]["adapter"] == "minimax-music"You can send follow-ups to the cloud agent here.
Comment @cursor review or bugbot run to trigger another review on this PR
Reviewed by Cursor Bugbot for commit b1c2510. Configure here.
| # missed the checkpoint). Do not start a second worker. | ||
| public = _public_minimax_music_job(existing_live) | ||
| public["replay"] = True | ||
| return public |
There was a problem hiding this comment.
Publish wipes reserved task identity
Medium Severity
submit_music_generation stores generation_id, candidate_id, command_id, and idempotency_key on the TaskRegistry row, then start_story_music_candidates_job publishes the MiniMax job to the same taskId. That publish upserts a replacement metadata object from job provenance (usually empty) and overwrites workflow/title, so the reserved identity does not survive on the canonical task.
Additional Locations (1)
Reviewed by Cursor Bugbot for commit b1c2510. Configure here.



Resumen ejecutivo
Qué cambia
Merge commit de
main(43f75b90, #143–#145) endevelopment(afdc0de7, #146).Única resolución: intro de
SLICE_QUEUE.md(base de integración + AGENT_QA_POLICY).Para qué sirve
Dejar
developmentcomo base de integración al día. A partir de aquí los PRs ordinarios apuntan adevelopment.Impacto para el usuario
Sin cambio de producto más allá de lo ya publicado en main (#143 música, #144 validación, #145 política QA) más la política de ramas ya en development (#146).
Riesgo
Estado
Summary
Authorized one-time sync: merge
mainintodevelopmentwith a merge commit (no squash, no force-push). Keep both the development integration-base paragraph and the agent QA merge wording.Overview
Parents:
afdc0de7(development / #146) and43f75b90(main / #145). Future ordinary PRs target development.Detailed changes
Backend
Music submission contract from #143.
UI and Wizard
None in the conflict resolution.
Data, provenance and compatibility
No data migrations.
Files and ownership
Conflict only in
docs/development/SLICE_QUEUE.md. Other files take the already-merged main or development side via default merge.Validation
pytestmusic + validate wrapper + development policy — 29 passedscripts/check_documentation_links.py— PASSscripts/verify_clean_repo.py— PASSBASE_SHA=origin/development bash scripts/check_code_health_pr_base.sh— Ratchet passedBASE_SHA=origin/development bash scripts/validate_local.sh— fast passedCode quality
No new production design. Incoming hotspot growth is from already-merged #143.
Risks and rollback
Revert this merge commit on development if needed. Do not force-reset. main is unchanged.
Dependencies and acceptance
User authorized this one-time merge of main into development. No auto-merge of later PRs.
Task cost report
External provider calls: 0. Real media generations: 0.
Note
Medium Risk
Changes the Story music job acceptance path in
_launch_runtime.pywith durable idempotency and concurrency rules; mistakes could duplicate jobs or strand reservations, though contract tests cover replay and races.Overview
Syncs development with landed work from main: an idempotent music submission contract wired into the async MiniMax Story song job route, plus fast vs
--fulllocal validation, agent QA policy docs, and PR template tweaks.Music (phase 4): New
music_submissionservice reserves command/generation/task/candidate IDs and a TaskRegistry row before the provider worker starts, with JSON dedup by idempotency key + spec hash (replay vs 409), Story destination checks by ID (not title), and optional retry/new-version intents.POST …/music-candidates/jobsnow calls that reservation first: replays return the live job or restart a single worker when the checkpoint is missing; concurrent replays must not spawn duplicate workers. The 202 body gains additive fields (generationId,commandId,candidateId,idempotencyKey,replay).Tooling & process:
validate_local.shdefaults to a fast pre-push path (explicitly not CI-equivalent) and adds--fullCI-like guards, full pytest, UI budget, logging, and fail-closed behavior when the code-health base SHA cannot be resolved;check_code_health_pr_base.shis stricter. Docs addMUSIC_SUBMISSION.md,AGENT_QA_POLICY.md, updatedLOCAL_VALIDATION.md/SLICE_QUEUE.md, and PR checklist items for validation scope and current HEAD CI/review evidence.Reviewed by Cursor Bugbot for commit b1c2510. Configure here.